Contrib > Timepicker visual tests #1895
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1895 +/- ##
======================================
Coverage 100% 100%
======================================
Files 416 416
Lines 8757 8757
Branches 1292 1292
======================================
Hits 8757 8757Continue to review full report at Codecov.
|
|
|
||
| <br/> | ||
|
|
||
| <h3> |
There was a problem hiding this comment.
I don't see any actual tests testing this third state.
There was a problem hiding this comment.
Yeah, I think this was copied from the demo template. The only visual difference seems to be that the input field is a type=hidden making it invisible. This doesn't really influence the visual state of the timepicker itself, so I don't think this is a case where we need visual test coverage. I'll remove it from the template.
| type="text" | ||
| [skyTimepickerInput]="timePickerExample1" | ||
| [timeFormat]="format12" | ||
| [(ngModel)]="selectedTime1" |
There was a problem hiding this comment.
When I try to run this locally I'm getting "Property 'selectedTime1' does not exist on type 'TimpickerVisualComponent'" errors. It does not appear that it exists so the error makes sense.
There was a problem hiding this comment.
Perhaps this wasn't run locally, the template seems to be a copy of the demo with the ids added. I'll clean it up and remove the non-existent references
| return SkyVisualTest | ||
| .setupTest('timepicker') | ||
| .then(() => { | ||
| element(by.css('.sky-dropdown-button')).click(); |
There was a problem hiding this comment.
I believe we will be opening the first dropdown here just like the first test as both dropdowns are on the same page and we aren't specifying which instance to grab.
There was a problem hiding this comment.
Yup, this is definitely not targeting specific dropdowns. I'll switch it
| @@ -0,0 +1,60 @@ | |||
| import { | |||
There was a problem hiding this comment.
This file needs renamed to timepicker.visual-spec.ts (notice the period instead of the dash after "timepicker"). Otherwise these tests will not run.
There was a problem hiding this comment.
Oh haha nice catch, I was wondering why it wasn't running for me 😛
|
@blackbaud-conorwright the selectors for the two test for the open state aren't giving us what we want. Below is what outputs for these tests and they are the same as the non-open screenshots. I believe we want to make sure that we have the full dropdown in the screenshot here.
|
|
The screenshot functionality is really finnicky about height 😕 |
changes moved from #1888 by @Remulus2006 to avoid history issues
Resolves: #1449